Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Lower System Parachain Existential Deposits #131

Merged
merged 5 commits into from
Jan 4, 2024
Merged

Lower System Parachain Existential Deposits #131

merged 5 commits into from
Jan 4, 2024

Conversation

joepetrowski
Copy link
Contributor

@joepetrowski joepetrowski commented Dec 31, 2023 •
edited

As referenced in a few discussions (e.g. forum), the Asset Hub Existential Deposit was set at genesis to 1/10th of the Relay Chain's. The goal however was always to lower it as the Relay Chain became more efficient at executing parachains to reflect the lower resource consumption on parachains and provide stronger incentives to prefer parachains for user-applications over the Relay Chain.

This PR lowers the Asset Hub existential deposits by a factor of ten, to 1/100th of the Relay Chain's ED, or 0.01 DOT.

For discussion/comment: Should all system paras have a lower ED, or just Asset Hub? On one hand, it is less confusing if they are all the same. On the other, we actually don't necessarily want to encourage lots of accounts on a chain like Bridge Hub.

@joepetrowski joepetrowski changed the title Lower Asset Hub Existential Deposit Lower System Parachain Existential Deposits Dec 31, 2023
@sourabhniyogi sourabhniyogi mentioned this pull request Jan 1, 2024
Closed
@ggwpez
Copy link
Member

ggwpez commented Jan 3, 2024

For discussion/comment: Should all system paras have a lower ED, or just Asset Hub? On one hand, it is less confusing if they are all the same. On the other, we actually don't necessarily want to encourage lots of accounts on a chain like Bridge Hub. For this PR, I've adjusted all of the system paras, but in separate commits.

Yea i think this is a good point. We can still reduce them if it should be a problem, but only reducing asset hub should already help.

@joepetrowski
Copy link
Contributor Author

joepetrowski commented Jan 4, 2024 •
edited

For discussion/comment: Should all system paras have a lower ED, or just Asset Hub? On one hand, it is less confusing if they are all the same. On the other, we actually don't necessarily want to encourage lots of accounts on a chain like Bridge Hub. For this PR, I've adjusted all of the system paras, but in separate commits.

Yea i think this is a good point. We can still reduce them if it should be a problem, but only reducing asset hub should already help.

Yeah, I'm inclined to just do Asset Hub since that's where we want DOT, and having DOT elsewhere should come with some particular motivation. It should be an easy and uncontroversial "win". For UX, wallets/apps should be able to read the ED of the chain they are on and inform users correctly. If there is compelling evidence that this causes problems, we can look at unifying them.

@joepetrowski
Revert "lower other eds"
4c15353 
This reverts commit 964cd80.
@bkchr bkchr merged commit 7000d24 into polkadot-fellows:main Jan 4, 2024
13 checks passed
@joepetrowski joepetrowski deleted the lower-para-ed branch January 4, 2024 12:21
@adamsteeber
Copy link

adamsteeber commented Jan 10, 2024 •
edited

Sorry if this isn't the best place for this or if it is too late being that it's merged already, but would it be possible to also decrease the assets.approvalDeposit on Asset Hub to 0.01 DOT as part of this merge as well?

@joepetrowski
Copy link
Contributor Author

Sorry if this isn't the best place for this or if it is too late being that it's merged already, but would it be possible to also decrease the assets.approvalDeposit on Asset Hub to 0.01 DOT as part of this merge as well?

The ApprovalDeposit is the existential deposit, so it's not only possible, it's done.
https://github.com/polkadot-fellows/runtimes/blob/v1.1.0/system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs#L263

@ggwpez
Copy link
Member

ggwpez commented Jan 14, 2024

Looks like this is missing a changelog entry and did therefore not appear in the release changelog.

@joepetrowski
Copy link
Contributor Author

Oops, adding one would trigger a new release though?

@ggwpez
Copy link
Member

ggwpez commented Jan 15, 2024 •
edited

I will just manually add it for now. PS: done

@bkchr
Copy link
Contributor

bkchr commented Jan 15, 2024

Oops, adding one would trigger a new release though?

No. You can open a pr to add it to the CHANGELOG.

@joepetrowski joepetrowski mentioned this pull request Jan 24, 2024
Merged
fellowship-merge-bot bot pushed a commit that referenced this pull request Jan 25, 2024
@joepetrowski
Actually Lower ED (#158)
4e41af1 
Since #115, chains do
not use their local `constants.rs` values, so
#131 did not have the
desired effect.

Would suggest a special Asset Hub 1.1.1 release to get this deployed as
soon as possible since a lot of applications were planning on it.
ggwpez pushed a commit that referenced this pull request Jan 25, 2024
@joepetrowski @ggwpez
Actually Lower ED (#158)
ab047c2 
Since #115, chains do
not use their local `constants.rs` values, so
#131 did not have the
desired effect.

Would suggest a special Asset Hub 1.1.1 release to get this deployed as
soon as possible since a lot of applications were planning on it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gilescope gilescope gilescope approved these changes

@xlc xlc xlc approved these changes

@bkchr bkchr bkchr approved these changes

@ggwpez ggwpez ggwpez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@joepetrowski @ggwpez @adamsteeber @bkchr @gilescope @xlc